Skip to content

refactor: replace context.WithCancel with t.Context#3183

Merged
julienrbrt merged 2 commits intoevstack:mainfrom
stringsbuilder:main
Mar 20, 2026
Merged

refactor: replace context.WithCancel with t.Context#3183
julienrbrt merged 2 commits intoevstack:mainfrom
stringsbuilder:main

Conversation

@stringsbuilder
Copy link
Contributor

@stringsbuilder stringsbuilder commented Mar 20, 2026 •
edited by coderabbitai bot
Loading

Overview

The addition of the Context method to Go's testing.T provides significant improvements for writing concurrent tests. It allows better management of goroutines, ensuring they properly exit and preventing issues like deadlocks and unfinished processes.

By using Context, errors and cancellations can be handled more effectively, making tests more robust and easier to reason about. This change also enables tighter integration between tests and the application code, especially for systems that span multiple concurrent components. Overall, it simplifies test code and enhances test stability and maintainability.

More info: golang/go#18368

Summary by CodeRabbit

  • Tests
    • Simplified test context management in async block retriever tests by using built-in test utilities instead of manual context creation and cleanup.

@stringsbuilder
refactor: replace context.WithCancel with t.Context
70d2a0b 
Signed-off-by: stringsbuilder <stringsbuilder@outlook.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 62c2857b-06e8-4f5b-a1e3-65d828564918

📥 Commits

Reviewing files that changed from the base of the PR and between d27d099 and 70d2a0b.

📒 Files selected for processing (1)
  • block/internal/da/async_block_retriever_test.go
📝 Walkthrough

Walkthrough

Test context handling is refactored to use Go's built-in t.Context() method instead of manually creating cancellable contexts with context.WithCancel(). The change affects four test functions in the async block retriever test suite with no modification to test logic or control flow.

Changes

Cohort / File(s) Summary
Test Context Refactoring
block/internal/da/async_block_retriever_test.go		 Replaced manual context cancellation setup (context.WithCancel() + deferred cancel()) with direct t.Context() usage across four test functions: TestAsyncBlockRetriever_SubscriptionDrivenCaching, TestAsyncBlockRetriever_CatchupFillsGaps, TestAsyncBlockRetriever_StopGracefully, and TestAsyncBlockRetriever_ReconnectOnSubscriptionError.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 No more cancel() calls to store,
Just t.Context() at the door,
Tests run cleaner, code so neat,
Context handling, now complete! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main refactoring: replacing context.WithCancel with t.Context in tests, and follows semantic commit conventions.
Description check ✅ Passed The description is well-structured, explains the rationale for the change, and includes a reference to relevant Go issue #18368, though the Overview section is complete.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@julienrbrt julienrbrt enabled auto-merge March 20, 2026 14:35
@codecov
Copy link

codecov bot commented Mar 20, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.14%. Comparing base (3a710a9) to head (8454c65).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3183      +/-   ##
==========================================
- Coverage   61.15%   61.14%   -0.02%     
==========================================
  Files         117      117              
  Lines       12082    12082              
==========================================
- Hits         7389     7387       -2     
- Misses       3867     3868       +1     
- Partials      826      827       +1
Flag Coverage Δ
combined 61.14% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@julienrbrt julienrbrt added this pull request to the merge queue Mar 20, 2026
Merged via the queue into evstack:main with commit 293a98e Mar 20, 2026
21 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@julienrbrt julienrbrt julienrbrt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stringsbuilder @julienrbrt